-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/first nib images #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to add protocols.yaml
with protocols definition e.g. https://github.com/blockjoy/blockvisor/blob/master/bv/tests/protocols.yaml
ethereum/ethereum-erigon/babel.yaml
Outdated
- key: erigon-goerli-archive | ||
archive_pointers: | ||
- pointer: !store_id ethereum-erigon-goerli-archive-v1 | ||
- pointer: !store_id ethereum-erigon-goerli-archive-v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't make sense to define new_archive: true
property if it have the same store_id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I wanted to achieve here is that whenever you select the tracing API, the dataset should point to the archive store rather than the full one. Can we configure this behavior more explicitly?
GRAFANA_PROM_API_KEY=${{ secrets.GRAFANA_PROM_API_KEY }} | ||
cache-from: type=gha | ||
cache-to: type=gha,mode=max | ||
tags: ghcr.io/blockjoy/${{ steps.version.outputs.image_name }}:${{ steps.version.outputs.image_tag }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also include a latest tag as we previously discussed. Trying to update all the dependencies with the hash is going to create a lot of work for every image update. Or we omit the hash and stick to the semantic version (i.e. we shouldn't ever have a version 1.1.1-hask8d and a 1.1.1-ja9dk1...two of the same sematic with different hashes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this mainly stemmed from the fact that I needed a way to bootstrap the pipeline since initially, no images were built that could satisfy the requirements of the next images. I ended up going for the commit of the hash and then updated the dependent Dockerfiles manually. We should only need the semver and only go for full release versioning, so don't allow pre-release versions on prod.
LE: I've moved the rest of my comment to the main PR discussion
clients/erigon/Caddyfile.template
Outdated
} | ||
|
||
http://{{ hostname }}{{ tld }}, https://{{ hostname }}{{ tld }}, *.blkjy.io, http:// { | ||
log { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log { | |
tls { | |
dns cloudflare {$CLOUDFLARE_API_KEY} | |
resolvers 1.1.1.1 | |
} | |
log { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include this, this will prevent the nodes from trying to use 127.0.0.1 for dns lookups, which doesn't work and helps gets certs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also there's a typo in the variable syntax
clients/reth/Caddyfile.template
Outdated
} | ||
|
||
http://{{ hostname }}{{ tld }}, https://{{ hostname }}{{ tld }}, *.blkjy.io, http:// { | ||
log { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log { | |
tls { | |
dns cloudflare {$CLOUDFLARE_API_KEY} | |
resolvers 1.1.1.1 | |
} | |
log { |
a2fe66b
to
ada7d4f
Compare
This is still work in progress, but while there still are things to test, it'll give you an idea of what the approach is.
Please fire away with feedback - it'll be appreciated as usual.